Stop passing raw API query options into the expression parser - #766

Type: bugfix

Issue

EnableQueryAttribute took $filter, $orderby and $select straight from the query string and handed them to System.Linq.Dynamic.Core — with no parsing restrictions, no whitelist of queryable fields, and no limit on expression size:

if (query.TryGetValue("$filter", out var filter))
    queryable = queryable.Where(filter.ToString());
if (query.TryGetValue("$select", out var select))
    queryable = queryable.Select($"new({select})");

The only guard anywhere was Configurations.MaxLimit on $top. With the default ParsingConfig the parser resolves types, honours the it / root context keywords and accepts new, so a caller could reach well past the model being queried. There was also no try/catch: because the filter runs in OnActionExecuted, a parse failure left the pipeline as a 500 instead of a 400, and $skip=abc was silently ignored rather than rejected.

Reproduce (module enabled, token with admin panel access): GET /api/Product?$filter=it.GetType().Assembly != null or $select= with anything that is not a field of the model.

Mitigating, and why this was not first in the queue: the module ships disabled ("Grand.Module.Api": false) and every controller sits behind AuthorizeApiAdmin.

Solution

$orderby and $select no longer reach the parser as client text at all. They are read as field lists, checked against the projected model and rebuilt in ApiQueryOptions. That removes the class of problem rather than filtering it, and matches what those two options mean in OData anyway.

$filter still needs a real expression, so it is fenced on three sides:

1. A ParsingConfig that resolves no types, disables the it / root context keywords, bans new, refuses to probe assemblies for additional types and denies Equals / ToString on object, backed by an empty IDynamicLinqCustomTypeProvider.
2. A 512-character limit, so a pathological expression cannot make the parser the slow part of the request.
3. A whitelist that accepts only public properties of the element type, seven cheap text methods (Contains, StartsWith, EndsWith, ToLower, ToUpper, Trim, Length, Equals) and the parser's own keywords. String literals are stripped before the whitelist runs, so a product named "Password" stays searchable.

$skip and $top now reject non-numeric and negative input. ApiQueryOptionException and ParseException map to 400 with a message naming the offending field.

Store scoping (item 2.2 of the same audit) is deliberately not in this PR. Verified in code: the API requires StandardPermission.ManageAccessAdminPanel, which no store-manager or vendor group holds by default (they get ManageAccessStoreManagerPanel / ManageAccessVendorPanel), so there is no narrower principal that could read another store through it. The DTOs carry no Stores / LimitedToStores to filter on — TableCollection<C>() deserializes entity documents into the DTO — and the only "current store" available to an API request comes from the Host header, so scoping the existing endpoints would break every global-admin integration. A store-scoped API belongs behind its own permission and an explicit, default-off setting.

Breaking changes

None to any published type. Behaviour changes for callers that were relying on the unrestricted parser:

• $filter naming anything outside the queried model, or using type/reflection syntax, now returns 400 instead of executing.
• $select accepts a plain field list only; expressions such as Name as Alias now return 400.
• $orderby accepts Field [asc|desc] only.
• Malformed $skip / $top return 400 instead of being ignored.

Every one of these was previously either an error waiting to happen or an unintended capability; no documented usage changes.

Testing

1. dotnet build ./GrandNode.sln
2. dotnet test src/Tests/Grand.Module.Api.Tests — 40 pass, 13 of them new.
3. Enable the module: set "Grand.Module.Api": true in FeatureManagement and BackendAPI.Enabled to true, then obtain a token via POST /token/create.
4. GET /api/Product?$filter=Name.Contains("shirt") → 200, filtered.
5. GET /api/Product?$filter=it.GetType().Assembly != null → 400, message names the rejected identifier. Same for $filter=PasswordHash != null and $select=it.GetType().
6. GET /api/Product?$select=Id,Name → 200, projection contains those two fields only.
7. GET /api/Product?$orderby=Name desc → 200, sorted; ?$orderby=Name sideways → 400.
8. GET /api/Product?$skip=-1 → 400; ?$top=99999 → capped at Configurations.MaxLimit.

🤖 Generated with Claude Code